Skip to content

Conversation

@flovent
Copy link
Contributor

@flovent flovent commented Jun 8, 2025

Fix false positives when copy assignment operator function in a template class returns the result of another assignment to *this, this check doesn't consider this situation that there will be a BinaryOperator for assignment rather than CXXOperatorCallExpr since this's type is dependent.

Closes #143237.

@llvmbot
Copy link
Member

llvmbot commented Jun 8, 2025

@llvm/pr-subscribers-clang-tidy

@llvm/pr-subscribers-clang-tools-extra

Author: None (flovent)

Changes

Fix false positives when copy assignment operator function in a template class returns the result of another assignment to *this, this check doesn't consider this situation that there will be a BinaryOperator for assignment rather than CXXOperatorCallExpr since this's type is dependent.

Closes #143237.


Full diff: https://github.com/llvm/llvm-project/pull/143292.diff

3 Files Affected:

  • (modified) clang-tools-extra/clang-tidy/misc/UnconventionalAssignOperatorCheck.cpp (+5-2)
  • (modified) clang-tools-extra/docs/ReleaseNotes.rst (+5)
  • (modified) clang-tools-extra/test/clang-tidy/checkers/misc/unconventional-assign-operator.cpp (+13)
diff --git a/clang-tools-extra/clang-tidy/misc/UnconventionalAssignOperatorCheck.cpp b/clang-tools-extra/clang-tidy/misc/UnconventionalAssignOperatorCheck.cpp
index afc4897eeb2ae..3fdaf9239f6af 100644
--- a/clang-tools-extra/clang-tidy/misc/UnconventionalAssignOperatorCheck.cpp
+++ b/clang-tools-extra/clang-tidy/misc/UnconventionalAssignOperatorCheck.cpp
@@ -66,8 +66,11 @@ void UnconventionalAssignOperatorCheck::registerMatchers(
                                 hasArgument(0, cxxThisExpr())),
             cxxOperatorCallExpr(
                 hasOverloadedOperatorName("="),
-                hasArgument(
-                    0, unaryOperator(hasOperatorName("*"),
+                hasArgument(0, unaryOperator(hasOperatorName("*"),
+                                             hasUnaryOperand(cxxThisExpr())))),
+            binaryOperator(
+                hasOperatorName("="),
+                hasLHS(unaryOperator(hasOperatorName("*"),
                                      hasUnaryOperand(cxxThisExpr())))))))));
   const auto IsGoodAssign = cxxMethodDecl(IsAssign, HasGoodReturnType);
 
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index a275d2ccfa004..394e18cd58f84 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -208,6 +208,11 @@ Changes in existing checks
   <clang-tidy/checks/misc/unused-using-decls>` check by fixing false positives
   on ``operator""`` with template parameters.
 
+- Improved :doc:`misc-unconventional-assign-operator
+  <clang-tidy/checks/misc/misc-unconventional-assign-operator>` check by fixing
+  false positives when copy assignment operator function in a template class
+  returns the result of another assignment to ``*this``(``return *this=...``).
+
 - Improved :doc:`misc-use-internal-linkage
   <clang-tidy/checks/misc/use-internal-linkage>` check by fix false positives
   for function or variable in header file which contains macro expansion and
diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc/unconventional-assign-operator.cpp b/clang-tools-extra/test/clang-tidy/checkers/misc/unconventional-assign-operator.cpp
index 74a22a7c083f4..9384d88c38fd0 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/misc/unconventional-assign-operator.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/misc/unconventional-assign-operator.cpp
@@ -163,3 +163,16 @@ struct TemplateTypeAlias {
   Alias3<TypeAlias::Alias> &operator=(double) { return *this; }
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: operator=() should return 'TemplateTypeAlias&' [misc-unconventional-assign-operator]
 };
+
+namespace issue143237 {
+template<typename T>
+struct B {
+  explicit B(int) {
+  }
+
+  B& operator=(int n) {
+    // No warning
+    return *this = B(n);
+  }
+};
+}

@github-actions
Copy link

github-actions bot commented Jun 8, 2025

⚠️ We detected that you are using a GitHub private e-mail address to contribute to the repo.
Please turn off Keep my email addresses private setting in your account.
See LLVM Discourse for more information.

Copy link
Contributor

@vbvictor vbvictor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@vbvictor vbvictor merged commit 6c153e5 into llvm:main Jul 6, 2025
10 checks passed
@flovent flovent deleted the clang-tidy-unconventional-assign-operator-FP branch July 6, 2025 12:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG|clang-tidy] false positive of misc-unconventional-assign-operator & cppcoreguidelines-c-copy-assignment-signature

4 participants